Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughPlain Changesbun update lockfile sync for self-referencing overrides
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/install/lockfile.rs`:
- Around line 911-934: Only patch overrides that are dollar-style
self-references: add/propagate a parse-time discriminator and check it before
mutating. Modify OverrideMap::parse_override_value to mark entries created from
"$name" (e.g., set a field on the override struct such as is_dollar_reference or
set an enum variant on the override value), then in the loop over
old.overrides.map.values_mut() only update override_dep.version when that marker
is set and the name_hash matches the root_by_hash entry; leave normal overrides
(e.g., direct version strings like "vite": "7.3.3") untouched.
- Around line 789-907: The loop that reconstructs updated literals ignores
PackageUpdateInfo.is_alias and thus rewrites aliased specs
(manager.updating_packages -> original_version_literal) into plain semver;
change the collecting step that builds updating (currently mapping to (Vec<u8>,
Vec<u8>)) to also include the is_alias flag from the PackageUpdateInfo, e.g.
(Vec<u8>, Vec<u8>, bool), and in the Pass 2 loop skip entries where is_alias is
true (or alternatively preserve the original "npm:<target>@" prefix when forming
buf via pinned_prefix logic); update the references in the loop that use (key,
original_literal) to match the new tuple and ensure aliased packages are not
rewritten before calling dependency::parse.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 546ce61b-b321-4a59-a9b0-bd5c330be6c7
📒 Files selected for processing (2)
src/install/lockfile.rstest/cli/install/bun-install-registry.test.ts
bun update (no args)$pkg overrides after bun update (no args)
|
Heads up — I independently opened #31754 for the same issue (#31748) before seeing this. Yours came first and is from the issue reporter, so I'm closing mine in favor of this one. One thing that might be worth folding in: the residual edge case you documented (a user writing the same literal in both |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/install/lockfile.rs`:
- Around line 782-792: The current pre_update_root_literals collapses multiple
root deps with the same PackageNameHash into one entry; change it to preserve
multiplicity by storing all literals for each root dependency slot instead of a
single Vec<u8>. For example, replace BunHashMap<PackageNameHash, Vec<u8>> used
in pre_update_root_literals with either BunHashMap<PackageNameHash,
Vec<Vec<u8>>> (and push each dep.version.literal.slice(buf).to_vec()) or a map
keyed by (dep.name_hash, dep.slot/index) so each dependency occurrence is
distinct; then update the downstream rewrite loops (the loops that currently
stop at first match when scanning root_deps_list and the override propagation
code) to consume every stored literal entry for a matching root dep rather than
only the first one, mirroring how preprocess_update_requests updates all
matching root entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2184a91b-27a2-4484-8520-cb8a428090df
📒 Files selected for processing (2)
src/install/lockfile.rstest/cli/install/bun-install-registry.test.ts
|
Folded in @robobun's Re: CodeRabbit feedback —
All |
|
Did one more self-review pass diffing against the closed #31754. @robobun's structure was leaner in two ways I'd missed:
Credit to @robobun for the cleaner shape. |
7f0c562 to
16b0ee0
Compare
16b0ee0 to
66fff02
Compare
|
Closing this one: all of the |
…ilter/--catalog, nested overrides, transitive update, and workspace fixes (#38333) Brings Bun's package manager to parity with pnpm for monorepo workflows, and fixes the bugs found while checking every command against pnpm's implementation, pnpm's test suites, pnpm's open issue tracker, pnpm's docs, npm's arborist fixtures, and — for `bun update` — running real pnpm and Bun side by side on the same projects. ### What does this PR do? #### New commands - **`bun dedupe [--check]`** — collapses duplicate versions in `bun.lock` onto the smallest set that still satisfies every dependent's range, using only versions already in the lockfile, then installs. Never downgrades a direct dependency unless that is the only way to drop a version; keeps patched versions (and anything needed to reach them, and says so); refuses to run on a lockfile that is behind `package.json`. `--check` exits 1 without writing. - **`bun prune [--production | --omit=…] [--dry-run] [--filter <ws>]`** — removes everything in `node_modules` that the lockfile does not put there; `--production` leaves exactly what `bun install --production` would. Hoisted and isolated layouts, Windows junctions and shims, workspace links, bundled deps; refuses when `package.json` and `bun.lock` disagree or when `node_modules` was laid out by a different linker; understands turbo-pruned checkouts. - **`bun pm licenses [--json] [--prod|--dev] [--long] [--filter <ws>]`** — installed packages grouped by license, with a `(dev)` marker and `paths`/`license`/`description` in `--json`. - **`bun audit fix [--latest] [--dry-run] [--json]`** — moves each vulnerable package to the lowest safe version its dependents accept, per installed instance; rewrites exact pins when that is the only way; `--latest` also rewrites your own declared ranges (root, workspace, catalog) so a semver-major fix can be taken, and every blocked or unfixable item is followed by the command that resolves it (`bun audit fix --latest`, `bun audit --ignore GHSA-…`); re-audits the tree it actually installed and reports/exits from that second response (npm's `_submitQuickAudit`), so an advisory that starts at the version it moved to is not missed; works across registries; security fixes bypass `minimumReleaseAge` with an annotation. `bun audit --json` honors `--audit-level`/`--ignore` for its exit code; `--omit` is honored by `audit` and `licenses`. #### `bun update` semantics (pnpm's model) - A bare `bun update` re-resolves **transitive** packages too — every edge moves to the newest version its own range (or dist-tag) allows, per dependent, so `bun.lock` no longer stays stale after an update; overrides/catalogs changed since the last install are honored. From a workspace member or `--filter`, only what the selected workspaces reach is re-resolved; from the root, everything. - `bun update <name>` reaches any depth, matches `npm:` aliases by real name, updates in place, never adds to `package.json`, and errors on a name nothing selected depends on. `-r`/`--filter` fan a named update out across workspaces. `--latest` never downgrades a locked version that is ahead of the tag, and `update <name> --latest` also refreshes that package's own dependencies. - Plain updates keep dist-tag literals and non-caret ranges (`*`, `1.x`, `^1 || ^2`) exactly as written and only move the lockfile; `--latest` rewrites them to the resolved version as before. `bun update -i` applies only what you selected. New: positional patterns (`bun update '@types/*'`), `--dev`/`--prod`/`--no-optional`, `-L`, `bun up`. - `package.json` is written after resolution and `bun.lock`'s declared ranges, overrides and catalogs are re-derived from the final `package.json`, replacing the per-command literal rewriting; a no-op update leaves the file byte-identical. #### Overrides - **Nested overrides** (#6608): npm's nested objects, yarn's `a/b` paths and pnpm's `a>b` selectors, applied to the direct parent→child edge; **version-scoped targets** (`"lodash@<4.17.21": "4.17.21"`, the shape `pnpm audit --fix` writes), matched against the dependent's declared range as pnpm does. Rules persist inside the `overrides` section and the file is stamped `lockfileVersion: 3` **only when such rules exist** — existing lockfiles are byte-identical. Flat overrides additionally fix `$ref` to workspace-member deps, catalog-valued rules going stale, and warn on pnpm's `-` / `pkg@` forms. #### Workspaces and filters - `bun add|remove|update … --filter <ws>` (also `-F`, also `bun install <pkg> --filter`) edits the selected workspaces' `package.json` files and **links only those workspaces**, like `bun install --filter`. Filters gain pnpm's relation selectors (`foo...`, `...foo`, `foo^...`, `...^foo`) and `{dir}` subtrees, for the install family **and** `bun run --filter`; `--filter` may precede the subcommand; every command warns about patterns that match nothing; `add`/`remove` no longer select the root implicitly. - `bun add <pkg> --catalog[=name]` reuses an existing catalog entry, keeps a range an explicit version fits, catalogs the range a package already declares, decides per target, and refuses workspace names and local paths; a plain `bun add` uses a default-catalog entry when one exists. A package defined in both `catalog` and `catalogs.default` is an error. `catalog:` peers of registry packages bind to the importer's copy instead of the root catalog. - `--frozen-lockfile` / `bun ci` on turbo-pruned monorepos: pruned-away workspaces are tolerated, a survivor depending on a pruned workspace is an error, catalog subsets are accepted, and an overrides/catalogs change is a frozen failure. #### One output vocabulary Every command here prints the install family's shapes: header, glyph rows (`+`/`-`/`↑`, dedupe's `↳ name old → new`), exactly one noun-first summary line with counts and a duration (`2 duplicate versions removed, 3 packages installed (checked 5 packages) [12ms]`, `N packages removed (checked C) [t]`), no-ops that say what was checked, remedies printed as copy-pasteable command lines, warnings as `warn:`, `--silent` printing nothing, and errors with their remedy together on stderr. Transitive and named updates render as the summary's `↑` rows (once per package; `--dry-run` prints the same rows plus `N packages would be updated`); dedupe reports after the install it triggers, so lifecycle-script output never splits it. A lockfile whose bytes did not change is no longer rewritten (`Saved lockfile` only prints on a real write; `--lockfile-only` no-ops print `Done! Checked N packages (no changes)`). This came out of running every command against fixtures and comparing with `install`/`add`/`remove` (95 findings, all fixed). #### Config precedence A project's `bunfig.toml` now beats any `.npmrc` (project or user-level) for the same key (npmrc files → bunfig's set fields → CLI); npmrc-only settings such as `//host/:_authToken` still attach to bunfig-declared registries, matched by host and path regardless of how either file spells the trailing slash. #### Lockfile migration - `package-lock.json`: rebuilt around a reachability walk that derives each resolution from the entry itself. Fixes `git+https://github.com/…` resolutions being written unparseably (the next install threw the lockfile away), root `bundleDependencies` migrating to an **empty** lockfile, lockfileVersion 1 (and npm's upcoming 4) making `bun install` exit 1 instead of resolving fresh, dependency-level bundles, `dependencies`+`optionalDependencies` double edges, unreferenced entries aborting the migration, duplicate packages for identical `name@version` at nested paths, lost `optionalPeers`, lost integrity when a bundled copy was seen first, and `overrides` not being carried over. All 57 of arborist's v2/v3 fixture projects are vendored and migrated under snapshot. - `pnpm-lock.yaml` v9: bare-hash `patchedDependencies`, snapshot aliases, `catalog:default`, recorded tarball URLs, git `path:`, multi-document files, `runtime:` entries, named registries, peer-suffixed keys chosen per importer, injected workspaces, manifest-only importer deps. #### Isolated linker - An existing store entry whose dependencies re-resolved (override, dedupe, update) now has its links refreshed on the next install (measured cost below). `bun prune` builds the same store the installer builds, so stale `name@version+<peerhash>` variants left by peer bumps are removed and a kept package's real entry never is (whether it was installed with full or `--production` features); on the hoisted linker, dedupe / audit fix / update delete the nested copies whose rows they collapsed instead of leaving the old copy loadable. - Blocked entries resume through per-entry intrusive waiter lists instead of a scan of every store entry after each completion (robobun's #25983/#28425 attempted this). Measured on the reporter's repro from #25799 (2,259 store entries) and a synthetic 6,425-entry monorepo, PR build vs merge-base build: main-thread CPU in the link phase drops 0.85 → 0.33 s and 5.5 → 0.85 s (the removed work grows quadratically); wall time is unchanged with spare cores and 16% / 26% faster pinned to one CPU, the CI/Docker shape in those reports. (The minute-long installs originally reported were peer resolution, fixed before this PR's base.) - Two pre-existing leaks surfaced by the new LSan-enabled tests are fixed: the header buffer of every authenticated registry request, and the per-entry lifecycle-script lists. #### Other bug fixes `bun add x@npm:pkg` writes a range; `bun add --trust a b` no longer drops `b` when `a` was already trusted; `bun add x --dev` no longer rewrote every group in `bun.lock`; `catalog:` peer hoisting; `dependency::Version::eql` treated all `catalog:` specifiers as equal; a `file:` package whose dependencies reach itself (its own name, an `npm:` alias under its own name, two link targets depending on each other — the shapes a `package-lock.json` migration produces, and #25202's `workspace:.` self-reference) hung `bun install` forever in the hoisting tree — the migration shapes now install, and #25202's literal shape now terminates with `Workspace dependency "foo" not found` rather than installing as npm does; a peer of a `file:` package that was only placed nested was also written to `optionalPeers` in a migrated bun.lock, so the next `--frozen-lockfile` failed and a plain install rewrote the lockfile; `catalog:` literals in `bun update`; alias output in the install summary; help/completions for everything above. #### Behavior changes to note in the release notes - `bun update` moves transitive packages; `bun update <name>` no longer adds an undeclared package (exit 1); `--production`/`--prod` on update means "only update `dependencies` and `optionalDependencies`" (a group filter like `--dev`, not the install flag) and `-r`+names with no match is an error; `-i` updates only the selection. - Project `bunfig.toml` overrides any `.npmrc`. - `bun install <pkg> --filter x` edits `x` (not the root); `bun add y --filter x` no longer installs a package named `x`; `add`/`remove --filter '*'` no longer includes the root. - A plain `bun add x` in a workspace whose default catalog lists `x` writes `catalog:`; `audit fix` may rewrite exact pins; `--frozen-lockfile --lockfile-only` writes nothing; overrides/catalog changes fail frozen installs. - One-time lockfile churn after upgrading for projects with `catalog:` peers or dead `pkg@range` override rows; lockfiles that use nested/scoped overrides are v3 and unreadable by older Bun (only when opted in). Turborepo, Nx and Dependabot have been checked; the needed upstream changes are open (nrwl/nx#36666 covers v2 and v3; vercel/turborepo#13740 accepts v3 and preserves the object rows through prune — turborepo main today parses v2 and rejects v3; dependabot needs nothing). Note v2 itself only exists on the 1.4 line. - `bun audit --json` keeps npm's contract: `--audit-level`/`--ignore` decide the exit code, the JSON document is the full registry report (closes #31013 as won't-change). Automatic removal of stale `node_modules` entries on plain `bun install` (#32974) is separate from this PR: `bun prune` is the manual form, and dedupe / audit fix / update now clean up the nested copies they collapse on the hoisted linker; #32974 should reuse prune's planner, and #29512 (sbom) is sequenced after this so it can build on `reachable.rs` instead of carrying its own walk. - Deliberately kept where we differ from pnpm: root `bun update` covers the whole workspace; dependents whose ranges allow follow a moved version (one copy, not two); `--latest` works on transitive names; `--no-save` touches neither file; prune deletion failures exit 1; audit requests stay per-registry. #### Performance Measured on a 1,113-package Next/Prisma/MUI app (PR build vs a PR build of the merge base, interleaved, plus canary and 1.3.14): every hoisted cell is within noise except no-op install, +0.8 ms (+2%, identical syscalls); isolated no-op is +1.9 ms (+3.9%) — the deliberate cost of re-checking existing entries' links every install rather than persisting a stamp file. Everything added is otherwise off the plain-install path (gated on the feature being used or on a diff), and id-indexed sets are bitsets. ### How did you verify your code works? ~1,100 new or ported test cases across the install suites (designed behavior, cases ported from pnpm's suites, pinning tests for pnpm bugs this implementation is immune to, arborist's fixtures, and the CI review findings), all `toStrictEqual`; the whole `test/cli/install` directory passes locally and the existing suites are unchanged except where a pre-existing expectation was deliberately changed (each listed above). `bun update` was additionally verified with a rerunnable differential harness that runs pnpm 11 and this branch on 26 scenario families against one registry and diffs the resulting resolutions edge by edge — after this PR only the deliberate differences above remain. Ecosystem: Turborepo, Nx and Dependabot were checked against the new lockfile output. Co-authored work absorbed with credit: @kjanat's #38190 (alias handling, co-author on the commit), @charpeni's #31143 and @crystalin's #34407 (both superseded), and the tests of the earlier `bun update` PRs (#31752 by @zlotnika, #33127, #36381, #36729, #38224). robobun's #34688 (folder-dependency cycles; its tests are lifted, co-author on the commit) and #37289 (migrated optionalPeers; its test is lifted, co-author on the commit) were fixed independently here and are closed by this PR. #28422's quadratic scan is fixed here as well (already closed). Related but not closed — `bun prune` gives these a manual fix while the automatic-cleanup asks stay open: #8662, #26305, #29793, #21216, #16176. Also related: #10930, #26970, #26751. Fixes #1343 Fixes #3605 Fixes #14719 Fixes #24122 Fixes #18612 Fixes #20238 Fixes #25826 Fixes #23615 Fixes #26973 Fixes #20593 Closes #31013 Fixes #28959 Fixes #28402 Fixes #27897 Fixes #26675 Fixes #10949 Fixes #18504 Fixes #13388 Fixes #24523 Fixes #6608 Fixes #19059 Fixes #16569 Fixes #8262 Fixes #11901 Fixes #13469 Fixes #25202 Closes #29664 Closes #31143 Closes #34407 Closes #34688 Closes #37289 Closes #38190 --------- Co-authored-by: Kaj Kowalski <info@kajkowalski.nl> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
Fixes #31748 (auto-closed as a duplicate of the still-open #13388).
"overrides": { "pkg": "$pkg" }is stored in the lockfile as a clone of the rootpkgdep, captured whenpackage.jsonis parsed.bun updatewith no args resolves a newer version and rewritespackage.json's range after resolution, so the cloned override keeps the pre-bump literal. The saved lockfile no longer matches thepackage.jsonwritten next to it, and the very nextbun install --frozen-lockfilefails withlockfile had changes, but lockfile is frozen.The fix is
Lockfile::refresh_self_referential_overridesinsrc/install/lockfile.rs, called frominstall_with_manageronceclean_with_loggerhas produced the lockfile that will be saved, and only forSubcommand::Update. It rewrites the literal of each$-ref override from the referenced root dep's resolved npm version, preserving the existing pin style (^/~/exact).To know which entries are
$-refs,OverrideMapnow records them explicitly inself_referentialas they are parsed (parse_override_valuereturns the referenced dep's name hash alongside the cloned dep). An earlier draft inferred this by comparing the override literal against the pre-update root literal, which misidentifies a user-authored literal override that happens to spell the same range. The map is not serialized — it is re-derived frompackage.jsonon every parse.npm:foo@…aliases andcatalog:references are skipped: their literals are not1.2.3-shaped, sowhich_version_is_pinnedreturnsPatchon the leadingn/cand the rewrite would strip the prefix. Those keep the existing behavior.Four tests in
test/cli/install/bun-install-registry.test.ts. The first reproduces the bug and fails on unpatched main with the exactlockfile had changes, but lockfile is frozenerror; the other three are negative-contract guards that fail only if the fix over-reaches — a literal override that differs from the root literal, a literal override that coincides with it, and a$-ref on an npm alias. Full file: 233 pass, 5 todo, 0 fail.